Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Body Parsing #53

Closed
wants to merge 48 commits into from
Closed

Feat: Body Parsing #53

wants to merge 48 commits into from

Conversation

shreyas-londhe
Copy link
Member

@shreyas-londhe shreyas-londhe commented Aug 29, 2024

Body Parsing Circuit Implementation

Changes

  • Implemented email_auth circuit with body parsing functionality
  • Added comprehensive tests for the new circuit


// Extract the command from the body
signal command_reveal[max_command_bytes];
component select_command_sub_array = SelectSubArray(max_body_bytes, max_command_bytes)(padded_body, command_idx, command_len);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is a definition of SelectSubArray?
I expect that it extracts a substring from command_idx to command_idx + command_len, but it has an attack vector because a relayer can choose any substring in the email body.
To prevent this, the circuit first needs to do a regex validation of padded_body for the regex <div id="zkemail">[^<>\r\n/]</div> and gets a revealed string for [^<>\r\n/] as command_reveal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It implies that an input command_len is unnecessary.

@shreyas-londhe shreyas-londhe marked this pull request as ready for review September 1, 2024 15:29
// * max_command_bytes - max number of bytes in the command
// * recipient_enabled - whether the email address commitment of the recipient = email address in the subject is exposed
// * is_qp_encoded - whether the email body is qp encoded
template EmailAuthWithBodyParsing(n, k, max_header_bytes, max_body_bytes, max_command_bytes, recipient_enabled, is_qp_encoded) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_qp_encoded

Can we fix the value of is_qp_encoded when operating the service?
Or do we need to deploy two circuits and choose which circuit is used for each email?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second option, because the flag will be turned on for email with big commands (which have soft-line breaks) and should be turned off for small ones as the proving time improves.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think a malicious relayer can choose an inappropriate circuit, which can be an attack vector?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the reason to turn off the flag is only for efficiency, I think we can always use a circuit with soft-line breaks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any attack surface here as the regex just won't pass if you send a body with QP encoding but don't remove those in the circuit. We can do the swapping thing imo

@Bisht13 Bisht13 changed the title Feat: Body Parsing in Email Auth Circuit Feat: Body Parsing Sep 2, 2024
@Bisht13 Bisht13 mentioned this pull request Sep 2, 2024
@Bisht13
Copy link
Member

Bisht13 commented Oct 1, 2024

Shifted commits to #59

@Bisht13 Bisht13 closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants